Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: [M3-8682] - Add 'Credit Card Expired' banner #11240

Merged

Conversation

harsh-akamai
Copy link
Contributor

Description 📝

Add a Credit Card Expired banner for accounts with expired credit cards

Changes 🔄

  • Implemented the banner

Preview 📷

image

How to test 🧪

Verification steps

  • Turn on MSW
  • Verify that the banner is visible

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@harsh-akamai harsh-akamai self-assigned this Nov 11, 2024
@harsh-akamai harsh-akamai requested a review from a team as a code owner November 11, 2024 11:02
@harsh-akamai harsh-akamai requested review from bnussman-akamai and abailly-akamai and removed request for a team November 11, 2024 11:02
@harsh-akamai harsh-akamai force-pushed the M3-8682-add-cc-expired-banner branch from 60c5fee to 530b239 Compare November 11, 2024 11:06
Copy link

github-actions bot commented Nov 11, 2024

Coverage Report:
Base Coverage: 87.37%
Current Coverage: 87.37%

</Button>
}
important
preferenceKey={'credit card expired'}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: This is probably doesn't matter, but it might be more common practice to make this - or _ separated rather than separated

Suggested change
preferenceKey={'credit card expired'}
preferenceKey={'credit-card-expired'}

Comment on lines 21 to 25
const isExpired = paymentMethods.some((paymentMethod: PaymentMethod) => {
const ccExpiry =
paymentMethod.type === 'credit_card' ? paymentMethod.data.expiry : null;
return ccExpiry && isCreditCardExpired(ccExpiry);
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: This might be a slightly cleaner way to represent this:

Suggested change
const isExpired = paymentMethods.some((paymentMethod: PaymentMethod) => {
const ccExpiry =
paymentMethod.type === 'credit_card' ? paymentMethod.data.expiry : null;
return ccExpiry && isCreditCardExpired(ccExpiry);
});
const isExpired = paymentMethods.some((paymentMethod) => {
if (paymentMethod.type === 'credit_card' && paymentMethod.data.expiry) {
return isCreditCardExpired(paymentMethod.data.expiry);
}
return false;
});

Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harsh-akamai see comments. I see in the ticket that it was mentioned

Are we only concerned that the default payment is the expiring card? Correct

which will allow us to use the existing account query and not add a new on on initial load.

also, can you please confirm we don't need to add a display for a card that's expiring soon?

@harsh-akamai
Copy link
Contributor Author

also, can you please confirm we don't need to add a display for a card that's expiring soon?

@abailly-akamai the reporter for this ticket mentioned that expiring soon is not the scope of this ticket.

@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🎉 445 passing tests on test run #5 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing445 Passing2 Skipped92m 45s

Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the changes - this looks good 👍

@abailly-akamai abailly-akamai merged commit f5beea7 into linode:develop Nov 18, 2024
22 of 23 checks passed
@harsh-akamai harsh-akamai deleted the M3-8682-add-cc-expired-banner branch November 18, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants